Repository navigation
fix(desktop): reset seeded new-workspace draft when the modal is dismissed - #6239
andyst-dev wants to merge 3 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThe new-workspace modal now clears its draft when dismissed. The full-page handoff preserves a seeded draft by disabling reset. The route clears the preserved draft when navigation leaves the create flow. Tests cover each lifecycle. ChangesNew workspace draft lifecycle
Estimated code review effort: 2 (Simple) | ~10 minutes Mergeability Score: ⚪ Minimal · up to The PR clears seeded workspace prompts when the modal is dismissed while preserving intended handoff behavior. No actionable merge-blocking risk remains beyond normal checks and review. Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThe PR resets the shared new-workspace draft whenever the modal store closes and adds regression coverage for ordinary dismissal. However, the same close action also participates in the full-page variant's handoff, where it now erases seeded context before navigation.
Confidence Score: 4/5This PR should not merge until the full-page new-workspace handoff preserves seeded prompts and linked issue or pull-request context. The new unconditional reset runs not only for dismissals but also immediately before the full-page variant navigates to a destination that relies on the same draft store, causing seeded entry flows to arrive empty. Files Needing Attention: apps/desktop/src/renderer/stores/new-workspace-modal.ts
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/stores/new-workspace-modal.ts | Adds draft cleanup to closeModal, but this also clears seeded state during the full-page variant's modal-to-page redirect. |
| apps/desktop/src/renderer/stores/new-workspace-modal.test.ts | Adds focused coverage for dismissal cleanup, but does not cover the existing closeModal-based full-page handoff. |
Sequence Diagram
sequenceDiagram
participant Seed as Seeded entry point
participant Draft as Draft store
participant Modal as Modal store
participant Router as Full-page redirect
participant Page as New Workspace page
Seed->>Draft: Store prompt or linked item
Seed->>Modal: openModal(projectId)
Modal->>Modal: Full-page variant handles open
Modal->>Draft: closeModal() resets draft
Modal->>Router: Navigate with projectId only
Router->>Page: Render /new-workspace
Page->>Draft: Read seeded context
Draft-->>Page: Empty draft
Prompt To Fix All With AI
### Issue 1
apps/desktop/src/renderer/stores/new-workspace-modal.ts:64
**Full-page handoff clears seeded draft**
When the full-page new-workspace variant handles a seeded setup-script, issue, or pull-request open, it calls `closeModal()` before navigating to `/new-workspace`. This reset erases the shared draft before the destination consumes it, causing the page to open without the seeded prompt or linked item.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "fix(desktop): reset seeded new-workspace..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // prompt the user never asked for. Every seed path calls | ||
| // resetDraft() before updateDraft(), so resetting here is safe | ||
| // and makes the dismiss the single cleanup point. | ||
| useNewWorkspaceDraftStore.getState().resetDraft(); |
There was a problem hiding this comment.
When the full-page new-workspace variant handles a seeded setup-script, issue, or pull-request open, it calls closeModal() before navigating to /new-workspace. This reset erases the shared draft before the destination consumes it, causing the page to open without the seeded prompt or linked item.
Knowledge Base Used: Desktop App (Electron main + renderer)
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/stores/new-workspace-modal.ts
Line: 64
Comment:
**Full-page handoff clears seeded draft**
When the full-page new-workspace variant handles a seeded setup-script, issue, or pull-request open, it calls `closeModal()` before navigating to `/new-workspace`. This reset erases the shared draft before the destination consumes it, causing the page to open without the seeded prompt or linked item.
**Knowledge Base Used:** [Desktop App (Electron main + renderer)](https://app.greptile.com/personal-org-6810/-/custom-context/knowledge-base/superset-sh/superset/-/docs/desktop-app.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/stores/new-workspace-modal.ts">
<violation number="1" location="apps/desktop/src/renderer/stores/new-workspace-modal.ts:64">
P1: The experimental full-screen handoff loses every seeded workspace draft: opening Configure (or an issue/PR workspace) closes the store modal before navigating, and this new reset wipes the prompt/links that the destination screen is supposed to consume. Keep the redirect handoff separate from dismissal cleanup, or preserve/reapply the draft across that navigation.</violation>
</file>
Architecture diagram
sequenceDiagram
participant User
participant UI as UI Components
participant Modal as NewWorkspaceModalStore
participant Draft as NewWorkspaceDraftStore
Note over User,Draft: #5372: Fix seeded prompt persistence on dismiss
alt Seeded Flow (e.g., V2SetupScriptCard "Configure")
UI->>Draft: resetDraft()
UI->>Draft: updateDraft({ prompt: "Write a setup script..." })
UI->>Modal: openModal(projectId)
Modal-->>UI: isOpen: true
User->>UI: Presses Esc / clicks outside
UI->>Modal: closeModal()
Modal->>Modal: set({ isOpen: false })
Modal->>Draft: resetDraft() // NEW: clears leftover prompt
Draft-->>Modal: prompt: ""
Modal-->>UI: isOpen: false
else Plain "New Workspace" open
UI->>Modal: openModal(projectId)
alt Draft was previously seeded and dismissed
Draft-->>UI: prompt: "" // cleared by closeModal
else No prior seed
Draft-->>UI: prompt: ""
end
UI-->>User: Empty composer ready
end
Note over Modal,Draft: Critical invariant: every seed path calls<br>resetDraft() before updateDraft(),<br>so closeModal() is the single cleanup point.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| // prompt the user never asked for. Every seed path calls | ||
| // resetDraft() before updateDraft(), so resetting here is safe | ||
| // and makes the dismiss the single cleanup point. | ||
| useNewWorkspaceDraftStore.getState().resetDraft(); |
There was a problem hiding this comment.
P1: The experimental full-screen handoff loses every seeded workspace draft: opening Configure (or an issue/PR workspace) closes the store modal before navigating, and this new reset wipes the prompt/links that the destination screen is supposed to consume. Keep the redirect handoff separate from dismissal cleanup, or preserve/reapply the draft across that navigation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/stores/new-workspace-modal.ts, line 64:
<comment>The experimental full-screen handoff loses every seeded workspace draft: opening Configure (or an issue/PR workspace) closes the store modal before navigating, and this new reset wipes the prompt/links that the destination screen is supposed to consume. Keep the redirect handoff separate from dismissal cleanup, or preserve/reapply the draft across that navigation.</comment>
<file context>
@@ -54,6 +55,13 @@ export const useNewWorkspaceModalStore = create<NewWorkspaceModalState>()(
+ // prompt the user never asked for. Every seed path calls
+ // resetDraft() before updateDraft(), so resetting here is safe
+ // and makes the dismiss the single cleanup point.
+ useNewWorkspaceDraftStore.getState().resetDraft();
},
</file context>
Round-1 review (ParadevAI#6239, greptile P1 + cubic P1): the full-page new-workspace test arm calls closeModal() before navigating to /new-workspace, where the destination screen consumes the seeded draft (setup-script prompt, linked issue/PR). The new dismissal cleanup wiped it, so the page opened empty. closeModal now takes { resetDraft?: boolean } (default true = the ParadevAI#5372 dismissal cleanup). The handoff opts out with { resetDraft: false }. 3 unit tests (dismiss clears, fresh open empty, handoff preserves). tsc 0 errors, biome clean.
|
Thanks — both points are addressed in
|
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/DashboardNewWorkspaceModal.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/DashboardNewWorkspaceModal.tsx:58">
P2: Abandoning the full-page create flow can still prefill the next New Workspace: this handoff opts out of cleanup, but the destination's close handler is a no-op and leaving it does not reset the global draft. Preserve the draft only for the active handoff, then clear it when the page is abandoned or scope the handoff draft to that route.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| // Keep the seeded draft (setup-script prompt, linked issue/PR): the | ||
| // /new-workspace page consumes it after the handoff, so this close | ||
| // must not run the #5372 dismissal cleanup. | ||
| closeModal({ resetDraft: false }); |
There was a problem hiding this comment.
P2: Abandoning the full-page create flow can still prefill the next New Workspace: this handoff opts out of cleanup, but the destination's close handler is a no-op and leaving it does not reset the global draft. Preserve the draft only for the active handoff, then clear it when the page is abandoned or scope the handoff draft to that route.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/DashboardNewWorkspaceModal.tsx, line 58:
<comment>Abandoning the full-page create flow can still prefill the next New Workspace: this handoff opts out of cleanup, but the destination's close handler is a no-op and leaving it does not reset the global draft. Preserve the draft only for the active handoff, then clear it when the page is abandoned or scope the handoff draft to that route.</comment>
<file context>
@@ -52,7 +52,10 @@ export function DashboardNewWorkspaceModal() {
+ // Keep the seeded draft (setup-script prompt, linked issue/PR): the
+ // /new-workspace page consumes it after the handoff, so this close
+ // must not run the #5372 dismissal cleanup.
+ closeModal({ resetDraft: false });
void navigate({
to: "/new-workspace",
</file context>
|
Thanks - fixed in The seeded draft is now scoped to the active full-page create flow:
Regression coverage verifies both sides of the lifecycle: handoff preservation and route-abandonment cleanup. Validation:
|
|
Heads-up: this PR no longer rebases cleanly — the new-workspace draft/modal code it touches was refactored on main. The Since the surrounding code has evolved past this change, I'm flagging it as obsolete unless a maintainer wants to revisit the draft-reset behaviour against the new modal. Happy to close it. |
Round-1 review (ParadevAI#6239, greptile P1 + cubic P1): the full-page new-workspace test arm calls closeModal() before navigating to /new-workspace, where the destination screen consumes the seeded draft (setup-script prompt, linked issue/PR). The new dismissal cleanup wiped it, so the page opened empty. closeModal now takes { resetDraft?: boolean } (default true = the ParadevAI#5372 dismissal cleanup). The handoff opts out with { resetDraft: false }. 3 unit tests (dismiss clears, fresh open empty, handoff preserves). tsc 0 errors, biome clean.
0a3cfdd to
b27d1c3
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
…issed Clicking Configure on the Setup-scripts sidebar card seeds the new workspace draft with a setup-script prompt and opens the modal. Dismissing the modal (Esc / outside click) called closeModal, which only flipped isOpen — the seeded prompt stayed in the persisted draft store, so the next '+ / New Workspace' opened pre-filled with a prompt the user never asked for (ParadevAI#5372). closeModal now resets the draft store. Every seed path already calls resetDraft() before updateDraft(), so the dismiss becomes the single cleanup point and no seed flow is affected. 2 unit tests: dismiss clears the seeded prompt; a fresh open after dismiss starts empty.
Round-1 review (ParadevAI#6239, greptile P1 + cubic P1): the full-page new-workspace test arm calls closeModal() before navigating to /new-workspace, where the destination screen consumes the seeded draft (setup-script prompt, linked issue/PR). The new dismissal cleanup wiped it, so the page opened empty. closeModal now takes { resetDraft?: boolean } (default true = the ParadevAI#5372 dismissal cleanup). The handoff opts out with { resetDraft: false }. 3 unit tests (dismiss clears, fresh open empty, handoff preserves). tsc 0 errors, biome clean.
b27d1c3 to
2809a52
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
Closing as superseded — the v2 new-workspace modal this PR modified was removed from |
Fixes #5372
Problem
Clicking Configure on the "Setup scripts" sidebar card opens the New Workspace modal seeded with a setup-script prompt. If you dismiss the modal (Esc / outside click) without creating a workspace, that prompt stays in the draft. The next time you click + / New Workspace, the composer still opens pre-filled with it — even though you never asked to configure setup scripts.
Root cause
Every seed path (
V2SetupScriptCard, GitHub issue/PR flows) doesresetDraft()+updateDraft(...)before opening the modal — but the dismiss path (closeModal) only flippedisOpen. The seeded prompt remained in the draft store, so the next plain "New Workspace" open consumed it as if the user had typed it.Fix
closeModalnow also resets the draft store. Since every seed path already resets before seeding, the dismiss becomes the single cleanup point:Validation
new-workspace-modal.test.ts— 2 unit tests:bun test: 2/2 pass ·tsc --noEmit: 0 errors · Biome: clean.Summary by cubic
Clears any seeded New Workspace draft on modal dismiss and preserves it only during the
/new-workspacehandoff. Previously dismiss left the seed in place; now dismiss wipes it, handoff keeps it, and leaving/new-workspaceclears it (fixes #5372).closeModal(options?: { resetDraft?: boolean })now resets the draft by default; passresetDraft: falsefor the/new-workspacehandoff (used byDashboardNewWorkspaceModalbefore navigation).resetNewWorkspaceDraftOnRouteLeaveand hooks it to the/new-workspacerouteonLeaveto clear the preserved seed on route leave.Written for commit 2809a52. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests